Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify String’s Extend<&str> implementation #27986

Merged

Conversation

chris-morgan
Copy link
Member

Reserving lower_bound bytes was just silly. It’d be perfectly reasonable
to have empty strings in the iterator, which could cause superfluous
reallocation of the string, or to have more than one byte per string,
which could cause additional reallocation (in practice it’ll balance
out). The added complexity of this logic is simply pointless, adding
a little bloat with no demonstrable advantage and slight disadvantage.

Reserving lower_bound bytes was just silly. It’d be perfectly reasonable
to have empty strings in the iterator, which could cause superfluous
reallocation of the string, or to have more than one byte per string,
which could cause additional reallocation (in practice it’ll balance
out). The added complexity of this logic is simply pointless, adding
a little bloat with no demonstrable advantage and slight disadvantage.
@rust-highfive
Copy link
Collaborator

r? @gankro

(rust_highfive has picked a reviewer for you, use r? to override)

@bluss
Copy link
Member

bluss commented Aug 25, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Aug 25, 2015

📌 Commit 81c1d14 has been approved by bluss

bors added a commit that referenced this pull request Aug 25, 2015
…tation, r=bluss

Reserving lower_bound bytes was just silly. It’d be perfectly reasonable
to have empty strings in the iterator, which could cause superfluous
reallocation of the string, or to have more than one byte per string,
which could cause additional reallocation (in practice it’ll balance
out). The added complexity of this logic is simply pointless, adding
a little bloat with no demonstrable advantage and slight disadvantage.
@bors
Copy link
Contributor

bors commented Aug 25, 2015

⌛ Testing commit 81c1d14 with merge 7472886...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants